fix(devnet): quiet status-registry probes and bump evm-upgrade to v1.20.2 - #209
Conversation
…20.2 - Add lookupStatusRegistryMnemonic, a silent registry lookup for probing infrastructure-key candidates that legitimately don't exist on a host (e.g. governance_key on a secondary validator), so probes no longer emit spurious WARN logs; readStatusRegistryMnemonic keeps warning for keys that are expected to be registered - Silence updateStatusRegistryAddress for untracked accounts: generated pre-evm-* fixtures live only in accounts-devnet.json, so a missing registry entry is the normal case - Remove unused appendStatusRegistryAccount - Fix Height format verb (%d -> %s) in supernode migration verification - Bump devnet-evm-upgrade target from v1.20.1 to v1.20.2 (via new devnet-upgrade-1202) - Add unit tests for status registry lookup helpers Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Line 717 in f6514bb
Separate the upgrade plan name from the binary version
When this target is run from the current post-v1.20.3 source tree, devnet-refresh-bin produces a binary stamped 1.20.3 (the CHANGELOG explicitly requires that binary for the on-chain v1.20.2 plan), but passing v1.20.2 here also makes upgrade-binaries.sh require the source binary's core version to be 1.20.2. Consequently, the flow can successfully schedule and halt the chain for v1.20.2 and then fail the binary swap with Source lumerad version mismatch, leaving the devnet halted; pass the plan name and expected binary version separately or otherwise permit the required 1.20.3 binary.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Addressed the review feedback and both failing CI paths in
Focused local checks passed, including the formerly failing system scenario twice and the JSON-RPC scenario three times. @codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bc2efb914a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Follow-up The two affected packages pass locally after the correction:
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2942b487f0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Addressed the latest occupied-upstream P2 in
Focused command tests and the JSON-RPC tests under the race detector pass. @codex review |
|
Follow-up @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1edc7280a3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Addressed the latest listener-collision P2 in
The full command package passes normally and under @codex review |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
🔵 Needs a closer look
It bundles a substantial, undescribed rework of production JSON-RPC startup port selection (plus other undocumented test-infra changes) that warrants human review beyond the stated devnet scope.
Pull request overview
This PR is primarily a devnet/test-stabilization change. As described, it quiets spurious WARN logs from the evmigration status-registry probes by splitting the lookup into a silent lookupStatusRegistryMnemonic (used for infrastructure-key candidates that legitimately don't exist on a given host) and the warning readStatusRegistryMnemonic (for keys expected to be registered), makes updateStatusRegistryAddress skip untracked accounts silently, removes the unused appendStatusRegistryAccount helper, corrects the Height format verb, and bumps the devnet-evm-upgrade flow to a v1.20.2 plan that installs a superseding v1.20.3 binary. Beyond the stated scope, the diff also reworks production JSON-RPC alias port selection in cmd/lumera/cmd/commands.go, adds a transient-error retry to an EVM integration test helper, refactors system-test node cleanup, and drops t.Parallel() from several tests.
Changes:
- Silent vs. warning status-registry lookups; silent skip of untracked accounts; removal of
appendStatusRegistryAccount;Heightverb fix (%d→%s, correct since the local JSON struct field is astring). - Devnet upgrade plumbing:
upgrade.shgains an optionalexpected-binary-versionarg,devnet-upgrade-1202installs the v1.20.3 binary for the v1.20.2 plan, anddevnet-evm-upgradetargets v1.20.2. - Undescribed additions: deterministic/exclusion-aware
reserveLoopbackAddrincommands.go, EVM tx-helper retry, system-testt.Cleanuprefactor, andt.Parallel()removals.
File summaries
| File | Description |
|---|---|
| devnet/tests/evmigration/status_registry.go | Splits lookup into silent/warning variants; silences untracked-account updates; removes unused append helper |
| devnet/tests/evmigration/status_registry_test.go | New unit tests for lookup/update helpers via a temp registry file |
| devnet/tests/evmigration/prepare.go | Uses the silent lookup when probing infrastructure-key candidates |
| devnet/tests/evmigration/migrate_validators.go | Corrects Height verb to %s (local JSON struct field is a string) |
| Makefile.devnet | Bumps devnet-evm-upgrade to v1.20.2 and passes v1.20.3 as the superseding binary |
| devnet/scripts/upgrade.sh | Adds optional expected-binary-version arg for plan-vs-binary version divergence |
| tests/scripts/devnet-makefile.bats | Adds a dry-run test asserting the v1.20.2 plan installs the v1.20.3 binary |
| cmd/lumera/cmd/commands.go | Undescribed: deterministic, exclusion-aware internal JSON-RPC loopback port selection |
| cmd/lumera/cmd/commands_test.go | Tests for the new loopback address selection logic |
| tests/integration/evmtest/tx_helpers.go | Undescribed: retries legacy tx send on transient "exceeds block gas limit" |
| tests/integration/evmtest/tx_helpers_test.go | Test for the transient-error classifier |
| tests/systemtests/system.go | Undescribed: moves node-result draining into t.Cleanup and stops the chain there |
| cmd/lumera/cmd/root_test.go, config_migrate_test.go | Undescribed: removes t.Parallel() from config/EVM-flag tests |
Review details
- Files reviewed: 14/14 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Summary
lookupStatusRegistryMnemonic, a silent status-registry lookup used when probing infrastructure-key candidates that legitimately do not exist on a host (for example,governance_keyon a secondary validator); keep warnings for keys expected to be registeredupdateStatusRegistryAddressfor untracked generatedpre-evm-*accounts and remove the unusedappendStatusRegistryAccounthelperHeightformat verb (%dto%s) in supernode migration verification errorsv1.20.2plan while independently validating and installing the requiredv1.20.3binaryexceeds block gas limitrejection for the fixed 21k integration-test transferScope and risk
The JSON-RPC change affects production CLI startup: it replaces an ephemeral internal upstream address with deterministic loopback selection while excluding the public JSON-RPC listener plus configured WS, metrics, API, gRPC, rate-limit proxy, and CometBFT listener ports. The transaction retry and node-cleanup changes are test-infrastructure stabilization only. Removing
t.Parallel()affects test scheduling only.Validation
v1.20.2plan /v1.20.3binary splitgo test -race ./cmd/lumera/cmd -count=18e41d466: lint, unit, integration, simulation, determinism, system, release build, and CodeQLRemaining manual validation
make devnet-evm-upgradeend-to-end on the shared devnet host🤖 Generated with Claude Code